🟡 Init-path SDK-default install runs outside hedgingStrategyLock — volatile fix only covers the read side

The early-return on this line is a volatile read of disableCrossRegionalHedging, but the rest of InitializePartitionLevelFailoverWithDefaultHedging (the SDK-default install at lines ~7000-7002) runs without hedgingStrategyLock. Interleaving with UpdatePartitionLevelFailoverConfigWithAccountRefresh (which does take the lock) produces this TOCTOU:

1. Init thread: volatile read sees flag=false, falls through past the early-return.
2. Refresh thread: takes lock, sets flag=true, runs ApplyHedgingStrategyForCurrentState — observes ConnectionPolicy.AvailabilityStrategy == null (init hasn't written yet), takes the no-op path, releases lock.
3. Init thread: writes ConnectionPolicy.AvailabilityStrategy = SDKDefaultCrossRegionHedgingStrategyForPPAF(...).

Final state: disableCrossRegionalHedging == true and ConnectionPolicy.AvailabilityStrategy != null. The RequestInvokerHandler.IsHedgingDisabledByGateway short-circuit added in this PR still suppresses hedging on the live request path (so the kill-switch contract holds), but the in-process invariant the rest of the file relies on is violated until the next genuine flag transition triggers another ApplyHedgingStrategyForCurrentState call. This is the same call site flagged earlier for the read side; commit 403d509f only addressed half the race.

Fix: wrap the install block (lines ~6980-7003) in lock (this.hedgingStrategyLock) { ... }, or refactor so all mutations of ConnectionPolicy.AvailabilityStrategy funnel through ApplyHedgingStrategyForCurrentState (which already owns the lock).

_{⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.}

🟡 Init-time stash assumes anything in ConnectionPolicy.AvailabilityStrategy is the customer's strategy

This branch in InitializeGatewayConfigurationReaderAsync unconditionally captures whatever is currently in ConnectionPolicy.AvailabilityStrategy into customerConfiguredAvailabilityStrategy. That happens to be true today because InitializePartitionLevelFailoverWithDefaultHedging runs after this method (at DocumentClient.cs:1118), so the only possible source at this point is the customer-supplied ClientOptions.AvailabilityStrategy.

Compare to ApplyHedgingStrategyForCurrentState, which carefully checks IsSDKDefaultStrategyForPPAF before stashing — precisely because the SDK should never persist its own auto-generated strategy as if it were the customer's intent. The init path should mirror that defensive check so a future reorder of OpenAsync doesn't silently stash an SDK-default object and then "restore" it as a phantom customer config when the operator toggles the flag back.

Fix:

if (this.disableCrossRegionalHedging
    && this.ConnectionPolicy.AvailabilityStrategy != null
    && !(this.ConnectionPolicy.AvailabilityStrategy is CrossRegionHedgingAvailabilityStrategy s && s.IsSDKDefaultStrategyForPPAF))
{
    this.customerConfiguredAvailabilityStrategy = this.ConnectionPolicy.AvailabilityStrategy;
    this.ConnectionPolicy.AvailabilityStrategy = null;
}

_{⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.}

🟡 ApplyHedgingStrategyForCurrentState "duplicate non-default strategy" branch silently drops customer config

This else if (!ReferenceEquals(this.customerConfiguredAvailabilityStrategy, currentStrategy)) branch logs a warning and then falls through to ConnectionPolicy.AvailabilityStrategy = null, throwing away the new strategy entirely. The accompanying comment ("re-entrant code path") acknowledges this is a "shouldn't happen" path, but silently losing customer configuration is the worst possible outcome for an operator-driven kill-switch — and it would surface as "hedging mysteriously stopped working after a flag toggle" with no clean diagnostic.

If this path is truly unreachable in production, either:

• Debug.Fail (or Assert) in DEBUG so test runs catch the regression immediately, and keep the warning trace for RELEASE, or
• Atomically swap: stash the new strategy into customerConfiguredAvailabilityStrategy before clearing, so a later toggle-off can restore something instead of the empty/SDK-default.

The current behavior — warn-and-silently-clear — is the worst of both worlds.

_{⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.}

🟢 <remarks> says "Caller must hold hedgingStrategyLock" but the method itself re-acquires it

The XML doc above the method declares "Caller must hold hedgingStrategyLock; the entry-point lock is reentrant so this..." — but the very next executable line is lock (this.hedgingStrategyLock). The actual contract is "method takes the lock; recursive entry from the (already-locked) UpdatePartitionLevelFailoverConfigWithAccountRefresh caller is fine because monitor locks are reentrant on the same thread." That's a different contract from what the doc states.

Today the only external caller (InitializePartitionLevelFailoverWithDefaultHedging from the init path) does not hold the lock — see F1 — so the wording will mislead a future maintainer into thinking the lock-free init call is a contract violation rather than a real bug. Either:

• Rewrite to: "Acquires hedgingStrategyLock. Safe to call from within UpdatePartitionLevelFailoverConfigWithAccountRefresh because monitor locks are reentrant.", or
• Make the contract real: have the method assume the lock is held, drop the inner lock, and fix the init-path caller to take it.

_{⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.}

🟡 latestPpafEnabled fallback reads SDK-mutated state as if it were Gateway truth

When accountProperties.EnablePartitionLevelFailover.HasValue == false but the hedging flag changed, this falls back to this.connectionPolicy.EnablePartitionLevelFailover to fill in the PPAF arg. That field is mutated by the SDK itself inside DocumentClient.UpdatePartitionLevelFailoverConfigWithAccountRefresh (line ~7045) on every prior fire of this same event. So:

• First fire: gateway says PPAF=true, hedging=false. We update connectionPolicy.EnablePartitionLevelFailover = true.
• Second fire: gateway doesn't include EnablePartitionLevelFailover (legitimate — operator only changed the hedging flag), and we fall back to the value we ourselves wrote last time, then feed it back into the callback which compares it to itself → ppafEnablementChanged is always false.

That's functionally correct today (no PPAF-flip side-effect runs when only hedging changed — exactly what we want), but it conflates "what the gateway said" with "what we last applied". A future writer to connectionPolicy.EnablePartitionLevelFailover (e.g., an admin API) would silently desync the cache. The hedging flag uses a dedicated lastKnownDisableCrossRegionalHedging field for exactly this reason — mirror the pattern: track a lastKnownEnablePartitionLevelFailover and fall back to that instead.

_{⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.}

🟡 lastKnownDisableCrossRegionalHedging advanced before Invoke — throwing handler leaves GEM/DocumentClient split-brain

Order on this line and the next:

this.lastKnownDisableCrossRegionalHedging = latestDisableHedging;   // <-- advance baseline
this.OnEnablePartitionLevelFailoverConfigChanged?.Invoke(...);     // <-- then notify

If the single subscriber (DocumentClient.UpdatePartitionLevelFailoverConfigWithAccountRefresh) throws — e.g., the new hedgingStrategyLock-protected reconcile hits an unexpected AvailabilityStrategy shape and the code in F3 falls into an uncovered branch, or a future caller adds a subscriber that throws — GEM's baseline has already moved. On the next refresh, the gateway still says the same value, disableHedgingFlagChanged evaluates false, no event fires, and DocumentClient.disableCrossRegionalHedging stays at its pre-throw stale value indefinitely (until the gateway flips the flag to a different value to break the diff).

For an operator kill-switch this is exactly the wrong failure mode — once you miss a transition, you can never recover the missed one on retry.

Fix: assign lastKnown after a successful invoke, or wrap with try/catch + revert:

bool previous = this.lastKnownDisableCrossRegionalHedging;
this.lastKnownDisableCrossRegionalHedging = latestDisableHedging;
try
{
    this.OnEnablePartitionLevelFailoverConfigChanged?.Invoke(latestPpafEnabled, latestDisableHedging);
}
catch
{
    this.lastKnownDisableCrossRegionalHedging = previous;  // allow retry next refresh
    throw;
}

_{⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.}

🟢 Trace fires only on the success branch — silent no-op leaves operators blind

InitializePartitionLevelFailoverWithDefaultHedging has multiple silent no-op paths: PPAF disabled mid-call, AvailabilityStrategy already non-null (after the customer restore branch above ran), or the flag flipped back to true between callbacks. The trace here only logs when the install succeeded. An operator toggling the kill-switch true → false and then watching logs has no positive confirmation that the SDK ended up in a known state on the no-strategy-installed branch — they will wonder whether hedging is back on, on at a different threshold, or still off.

Add a TraceWarning on the else branch with the observed state (EnablePartitionLevelFailover, AvailabilityStrategy?.GetType().Name, current disableCrossRegionalHedging) so the post-toggle state is always recorded:

if (this.ConnectionPolicy.AvailabilityStrategy != null) { TraceInformation(...applied...); }
else { TraceWarning("DocumentClient: Hedging re-enable did not install a strategy (PPAF={0}, existing={1}, killSwitch={2})", ...); }

_{⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.}

💬 Per-request short-circuit's two-field read is not atomic — long comment overstates consistency

This handler does two unrelated reads in sequence:

if (this.client.DocumentClient.IsHedgingDisabledByGateway) return null;       // volatile bool
AvailabilityStrategy strategy = request.RequestOptions?.AvailabilityStrategy
        ?? this.client.DocumentClient.ConnectionPolicy.AvailabilityStrategy;  // plain field

A reader can observe IsHedgingDisabledByGateway == false (snapshot before a refresh) and then ConnectionPolicy.AvailabilityStrategy == null (snapshot after the refresh cleared it). Outcome: no hedging while a strategy was "active from the customer's perspective". That's actually fine for the kill-switch's correctness contract (kill-switch wins, never the other way), but the long XML doc on disableCrossRegionalHedging (line ~31) and the comment on IsHedgingDisabledByGateway (line ~317) imply the per-request view is consistent across both fields. It isn't.

Either:

• Tighten the doc to "the only consistency guarantee across the two reads is: when the volatile read sees true, hedging is off — the reverse is not symmetric", or
• Snapshot ConnectionPolicy.AvailabilityStrategy to a local before the flag check so the precedence becomes "kill-switch wins regardless of which snapshot we took".

Tightening the doc is the lighter touch and matches actual behavior.

_{⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.}

🟡 Hard-coded "disableCrossRegionalHedging" JSON literal is not pinned by a round-trip test

The TODO above this attribute correctly flags that Constants.Properties is the established source of truth for these names, and the future Direct package update will route through it. In the meantime, the only protection against a typo or a stealth rename is the unit tests — but the new tests in GatewayHedgingOverrideTests.cs only deserialize JSON containing the literal, and the modifications to CosmosJsonSerializerUnitTests.cs / CosmosSerializerCoreTests.cs only assert serializer ordering of the property among siblings. A future PR that renames the field to DisableCrossRegionalHedgingFlag and lockstep-updates the serializer expectation would compile, pass all tests, and silently break wire-binding.

Add one serialize round-trip test (separate from the order-only ones already updated) that pins the exact JSON key:

[TestMethod]
public void DisableCrossRegionalHedging_SerializesWithExactJsonKey()
{
    var props = new AccountProperties { DisableCrossRegionalHedging = true };
    string json = JsonConvert.SerializeObject(props);
    Assert.IsTrue(json.Contains("\"disableCrossRegionalHedging\":true"),
        "JSON property name is the wire contract with Gateway and must not change without a coordinated server-side rename.");
}

_{⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.}

💬 RequestInvokerHandler_FlagTrue_PerRequestStrategy_ReturnsNull sets the flag bypassing the production reconcile path

This test uses the new DisableCrossRegionalHedgingForTests = true accessor, which only mutates the volatile bool — it never invokes UpdatePartitionLevelFailoverConfigWithAccountRefresh or ApplyHedgingStrategyForCurrentState. So it validates the RequestInvokerHandler short-circuit in isolation, which is valuable. But the in-code assertion message claims to verify "Gateway operator override must take absolute precedence over per-request AvailabilityStrategy" — that overstates what is being tested: a test-only setter is not the production path the operator actually exercises.

Add a sibling test that:

1. Constructs a DocumentClient with a customer-configured AvailabilityStrategy.
2. Drives the flag via client.UpdatePartitionLevelFailoverConfigWithAccountRefresh(latestIsEnabled: true, latestDisableCrossRegionalHedging: true) (the actual production callback).
3. Sends a request with a per-request RequestOptions.AvailabilityStrategy set.
4. Asserts the handler returns null.

That covers the production reconcile + per-request precedence path end-to-end, locking in the contract the current test only asserts in isolation.

_{⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.}